Add retry around wclayer operations for process isolated containers - #1091
Merged
Merged
Conversation
Danny Canter (dcantah)
force-pushed
the
retry-layerops
branch
2 times, most recently
from
August 2, 2021 23:54
e0edb8f to
0df4d76
Compare
Contributor
Author
|
Should hopefully help #919 |
Danny Canter (dcantah)
force-pushed
the
retry-layerops
branch
2 times, most recently
from
August 3, 2021 04:58
b483e89 to
76cd63c
Compare
Contributor
Author
|
Scott Brender (@msscotb) I did when trying to get the PrepareLayer issue to reproduce 😆 I got ERROR_DEVICE_NOT_CONNECTED |
Contributor
Author
|
Scott Brender (@msscotb) Any other feedback for this? |
This change adds a simple retry loop to handle some behavior on RS5. Loopback VHDs used to be mounted in a different manor on RS5 (ws2019) which led to some very odd cases where things would succeed when they shouldn't have, or we'd simply timeout if an operation took too long. Many parallel invocations of this code path and stressing the machine seem to bring out the issues, but all of the possible failure paths that bring about the errors we have observed aren't known. On 19h1+ this retry loop shouldn't be needed, but the logic is to leave the loop if everything succeeded so this is harmless and shouldn't need a version check. Signed-off-by: Daniel Canter <dcanter@microsoft.com>
Danny Canter (dcantah)
force-pushed
the
retry-layerops
branch
from
August 5, 2021 02:08
76cd63c to
01b9911
Compare
| } | ||
|
|
||
| defer func() { | ||
| if err != nil { |
Contributor
There was a problem hiding this comment.
Doesn't err need to be set to PrepareLayer result for the deferred DeactivateLayer to execute?
Contributor
Author
There was a problem hiding this comment.
Nope, if you have a named return value, e.g. (err error) then the return value of line 107 or the PrepareLayer call will get assigned to err after completion. So when defer runs it will have the return value of PrepareLayer to check against.
Here's a quick example: https://play.golang.org/p/cID3RHPwl88
Scott Brender (msscotb)
approved these changes
Aug 5, 2021
|
Danny Canter (@dcantah) Scott Brender (@msscotb) Will this fix be included in moby? We have seen this failure in docker many times. |
Maksim An (anmaxvl)
pushed a commit
to anmaxvl/hcsshim
that referenced
this pull request
Nov 17, 2021
Related work items: microsoft#930, microsoft#962, microsoft#1004, microsoft#1008, microsoft#1039, microsoft#1045, microsoft#1046, microsoft#1047, microsoft#1052, microsoft#1053, microsoft#1054, microsoft#1057, microsoft#1058, microsoft#1060, microsoft#1061, microsoft#1063, microsoft#1064, microsoft#1068, microsoft#1069, microsoft#1070, microsoft#1071, microsoft#1074, microsoft#1078, microsoft#1079, microsoft#1081, microsoft#1082, microsoft#1083, microsoft#1084, microsoft#1088, microsoft#1090, microsoft#1091, microsoft#1093, microsoft#1094, microsoft#1096, microsoft#1098, microsoft#1099, microsoft#1102, microsoft#1103, microsoft#1105, microsoft#1106, microsoft#1108, microsoft#1109, microsoft#1115, microsoft#1116, microsoft#1122, microsoft#1123, microsoft#1126
Prince Pereira (princepereira)
pushed a commit
to princepereira/hcsshim
that referenced
this pull request
Aug 29, 2024
Add retry around wclayer operations for process isolated containers
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This change adds a simple retry loop to handle some behavior on RS5. Loopback VHDs
used to be mounted in a different manor on RS5 (ws2019) which led to some
very odd cases where things would succeed when they shouldn't have, or we'd simply
timeout if an operation took too long. Many parallel invocations of this code path
and stressing the machine seem to bring out the issues, but all of the possible failure
paths that bring about the errors we have observed aren't known.
On 19h1+ this retry loop shouldn't be needed, but the logic is to leave the loop if everything succeeded so this is harmless
and shouldn't need a version check.
Signed-off-by: Daniel Canter dcanter@microsoft.com